Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add webrtc #179

Merged
Merged

Conversation

ckousik
Copy link
Contributor

@ckousik ckousik commented Jul 1, 2022

Adds an x-webrtc protocol which holds the 32 byte hex encoded DTLS fingerprint received by the server. This is based on melekes/rust-multiaddr@c88f5f4 and is for supporting the Go implementation of the spec described in libp2p/specs#412 .

@marten-seemann
Copy link
Contributor

Wasn't the plan to use /webrtc/certhash/xxx?

@mxinden
Copy link
Member

mxinden commented Jul 11, 2022

Wasn't the plan to use /webrtc/certhash/xxx?

👍 Followed up on https://github.com/libp2p/specs/pull/412/files#r917573385.

@ckousik
Copy link
Contributor Author

ckousik commented Jul 12, 2022

@ckousik ckousik changed the title Add x-webrtc Add webrtc Jul 12, 2022
transcoders.go Outdated Show resolved Hide resolved
protocols.go Outdated Show resolved Hide resolved
@ckousik ckousik requested a review from marten-seemann July 12, 2022 14:16
Copy link
Contributor

@marten-seemann marten-seemann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Can you add the code point to https://github.com/multiformats/multiaddr?

@marten-seemann
Copy link
Contributor

CI is unhappy about this PR. I think you'll need to run gofmt.

@ckousik ckousik requested a review from marten-seemann July 12, 2022 15:12
@ckousik
Copy link
Contributor Author

ckousik commented Jul 12, 2022

@marten-seemann Seems like 0x115 (277) is already used by p2p-stardust https://github.com/multiformats/multiaddr/blob/master/protocols.csv#L33 . Should I change the code for webrtc to 280 or some other nearby number?

@marten-seemann
Copy link
Contributor

Yes, makes sense. Just pick a number.

@ckousik
Copy link
Contributor Author

ckousik commented Jul 12, 2022

LGTM.

Can you add the code point to https://github.com/multiformats/multiaddr?

Made a PR multiformats/multiaddr#131 . These should be good to merge.

@marten-seemann marten-seemann merged commit 696f50a into multiformats:master Jul 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants